Skip to content

feat: declarative default option for reactive props - #575

Closed
vivek7405 wants to merge 5 commits into
mainfrom
feat/simpler-reactive-prop-dx
Closed

feat: declarative default option for reactive props#575
vivek7405 wants to merge 5 commits into
mainfrom
feat/simpler-reactive-prop-dx

Conversation

@vivek7405

Copy link
Copy Markdown
Collaborator

Closes #531

Summary

Researched whether the reactive-prop DX can be simplified below the static properties + declare + constructor-default pattern while holding the no-build + erasable-TS + no-decorator invariants. Conclusion: the declare-typed accessor is irreducible, but the constructor can go for the common case via a new declarative default: option.

// before (3 pieces)
static properties = { count: { type: Number } };
declare count: number;
constructor() { super(); this.count = 0; }

// after (2 pieces)
static properties = { count: { type: Number, default: 0 } };
declare count: number;

A function default is invoked per instance (fresh object/array, no shared reference); an applied attribute still overrides it.

Why the other options were rejected (evaluation in a comment below)

  • accessor keyword: a runtime syntax error on Node 26 / V8 (verified) and the type stripper leaves it intact (it is not a type), so it cannot work without a build step. Also does not auto-register reactivity without a (banned) decorator.
  • prop() field helper: a class-field initializer clobbers the prototype accessor (verified), so it stores a box, not a reactive value, with no registration timing hook.
  • infer instance types from static properties: TS has no decorator-free way to add typed instance members from a static value.
  • codegen via webjs types: reintroduces a generate/watch step and drift, worse than the status quo.

Test plan

  • Unit: default seeds value (no constructor), falsy defaults applied, function-default fresh per instance, attribute overrides default, reflect-on-connect (packages/core/test/lifecycle/component-lifecycle.test.js).
  • Counterfactual: new tests fail when the runtime change is reverted.
  • Check-rule tests still pass (message updated to mention default).
  • Docs: lit-muscle-memory-gotchas.md (the deliberate Lit divergence), AGENTS.md, docs site components page.

Doc-surface walkthrough and the full options evaluation will be posted as comments as the work lands.

Add a `default:` field to `static properties` declarations so the common
case no longer needs a constructor purely to seed a value. A function
default is invoked per instance (fresh object/array, no shared reference);
an applied attribute still overrides it.

This is the one DX improvement for the static-properties + declare pattern
that holds the no-build + erasable-TS + no-decorator invariants (#531): the
`accessor` keyword is a runtime syntax error and is not erasable, and a
prop() field helper is clobbered by class-field semantics, so neither can
avoid a build step. The declare-typed accessor itself is irreducible.
@vivek7405 vivek7405 self-assigned this Jun 18, 2026
t added 2 commits June 18, 2026 14:16
…) divergence

Sync the reactive-prop DX across surfaces for the new `default` option:
AGENTS.md prop-options sentence, the components deep-dive options table,
and the docs-site components example (now constructor-free).

Rewrite the lit-muscle-memory-gotchas `@property()` entry into the full
deliberate-divergence rationale (#531): decorators are non-erasable so
they would force a build step, the `accessor` keyword is a runtime syntax
error and is not erasable, a prop() field helper is clobbered by
class-field semantics, so the `declare`-typed accessor is irreducible and
`default` is what removes the constructor.
@vivek7405

Copy link
Copy Markdown
Collaborator Author

Design rationale: why the declare accessor is irreducible, and what default buys

I went through every option for getting below the static properties + declare + constructor-default pattern, against the three hard constraints (no decorators, erasable-TS-only, no build step). Recording the evaluation here so the decision is on the PR.

The constraint that forces all of this. A class-field initializer (name = 'x') is defined with [[Define]] semantics after super() returns, so it overwrites the prototype/instance accessor the framework installs in the constructor. I verified this on the target runtime: a subclass field turns the own descriptor's get into undefined and stores a plain value, so the reactive setter never runs again. That is the whole reason a plain field cannot carry a reactive prop, and it sinks the field-based options below.

Options evaluated:

  1. The accessor keyword (TC39 auto-accessors). Tempting because auto-accessors are not legacy decorators. Two independent failures, both verified on Node 26 / V8: (a) accessor x = 1 is a runtime syntax error without the decorators proposal, and the type stripper leaves it intact because it is not a type, so it crashes at module load with no build step to lower it; (b) even where it parses, a bare accessor does not register the property as reactive (no observedAttributes, no reflect, no requestUpdate) without a decorator, which is the banned part. Rejected.

  2. A prop() field helper (name = prop<string>('x')). Same class-field clobber as a plain field: it stores the helper's return value as a data property, not a reactive accessor, and there is no decorator-free hook that fires at the right time to convert it back. Rejected.

  3. Infer instance member types from static properties. TypeScript has no decorator-free mechanism to add a typed instance member to this from a static runtime value. A generic base or mixin cannot add arbitrarily-named members either. So the type has to be stated once somewhere, which is exactly what declare does. Rejected.

  4. Codegen via webjs types. Could parse static properties and emit a declaration-merging .d.ts, but it reintroduces a generate/watch dependency and a drift surface for a one-line-per-prop saving. Worse than the status quo. Rejected.

Conclusion. The declare line is irreducible given the constraints. What is NOT irreducible is the constructor, whose only job in the common case is to seed a default. So I added a declarative default option to the property declaration, bringing the common case to two lines:

static properties = { count: { type: Number, default: 0 } };
declare count: number;

A function default is invoked per instance, so object/array defaults are fresh per element (the same shared-reference trap a student = {…} field would have). An applied attribute still overrides it, and the value is in the SSR first paint (verified), so it stays progressive-enhancement-safe. This is the best DX the constraints allow, and the deliberate divergence from Lit's @property() one-liner is now documented in agent-docs/lit-muscle-memory-gotchas.md.

t added 2 commits June 18, 2026 14:22
A class-field initializer is re-evaluated per construction, so it is fresh
per instance and never the shared-reference trap (its trap is accessor
clobbering). The actual shared-reference footgun is a LITERAL object/array
`default`, evaluated once in `static properties` and shared by reference
across instances. Rewrite the gotchas + components-deep-dive text to warn
about the literal form and steer objects/arrays to a function `default`,
and lock the contract with a test.

@vivek7405 vivek7405 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went over the whole change end to end. The runtime branch is sound: the default lands in the backing store exactly like the existing initial capture, an applied attribute / .prop= hydration still overrides it, and state props stay out of observedAttributes. The function-vs-literal default semantics are right and the SSR first paint carries the value, so it is PE-safe. One coverage gap I noticed and closed: nothing asserted a reflect: true default reflecting in the SSR markup (only the client-connect leg). Added that SSR-reflect test.

@vivek7405 vivek7405 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second pass, focused on the default-value semantics and the doc claims. Found one real inaccuracy in my own prose (now fixed): I had described the function-default as avoiding 'the trap a field initializer would create', but a class-field initializer is re-evaluated per construction, so it is fresh per instance and never a shared-reference trap (its trap is accessor clobbering). The genuine shared-reference footgun is a LITERAL object/array default, evaluated once in static properties and shared by reference. Rewrote the gotchas entry and the components deep-dive to warn about the literal form and steer objects/arrays to a function default, and added a test that locks the shared-literal contract. Everything else (state+default, hydration override order, hasChanged bypass parity, an error thrown in a function default) traced clean.

Comment thread agent-docs/lit-muscle-memory-gotchas.md
vivek7405 pushed a commit that referenced this pull request Jun 23, 2026
… aside

The default + custom attribute options are now implemented, but defaults are
documented via the constructor (the recommended webjs idiom), not the default
option. Sweep all surfaces (AGENTS, agent-docs/components +
lit-muscle-memory-gotchas, docs site components/task/troubleshooting, blog +
scaffold rule files) to constructor-set defaults, keep a one-line lit-parity
note that the default option also exists, and update the no-longer-static
-properties error message to point at the constructor.

Refs #575

Claude-Session: https://claude.ai/code/session_01WnvcTojG7tYqmnmf4enSv3
vivek7405 pushed a commit that referenced this pull request Jun 23, 2026
…falsy default

Self-review round 1: the no-static-properties / reactive-props-no-class-field
rule messages (and the fix text) in check.js still told agents to set defaults
via the `default` option, contradicting the constructor-first sweep. Point
them at the constructor. Add a falsy-default test (0 / false / '') pinning the
`!== undefined` guard.

Refs #575

Claude-Session: https://claude.ai/code/session_01WnvcTojG7tYqmnmf4enSv3
vivek7405 pushed a commit that referenced this pull request Jun 23, 2026
applyAttrsToInstance mapped a source attribute to its property by name /
camelCase only, ignoring the custom `attribute` option, so a parent-rendered
<el is-open> left the real prop unset in the SSR'd first paint (the client
attributeChangedCallback self-corrected after hydration, but the JS-off
contract was wrong). Resolve via the same (d.attribute || hyphenate(k)) rule
the client uses. SSR tests cover default-in-HTML + custom-attribute inbound +
a counterfactual.

Refs #575

Claude-Session: https://claude.ai/code/session_01WnvcTojG7tYqmnmf4enSv3
vivek7405 pushed a commit that referenced this pull request Jun 23, 2026
…option

extractStaticProperties always kebab-cased the property name for the HTML
attribute, so a prop with a custom `attribute` option got the wrong name in
template attribute completions / hover. Read the option (propAttrName), matching
the runtime. Re-vendored the nvim copy src; test covers a custom-attribute
completion.

Refs #575

Claude-Session: https://claude.ai/code/session_01WnvcTojG7tYqmnmf4enSv3
@vivek7405

Copy link
Copy Markdown
Collaborator Author

Superseded by #672. The declarative default option is implemented there on the current factory DX (this PR was built on the pre-#597/#599 static-properties/declare model and conflicts). #672 also adds the custom attribute option and documents the constructor as the recommended way to set defaults (with default kept as a lit-parity option). Closing as superseded.

@vivek7405 vivek7405 closed this Jun 23, 2026
vivek7405 pushed a commit that referenced this pull request Jun 23, 2026
…ption

Lock the precedence (default applied in super() is the fallback; a constructor
assignment after super() wins) so a future change cannot silently flip it.

Refs #575

Claude-Session: https://claude.ai/code/session_01WnvcTojG7tYqmnmf4enSv3
vivek7405 added a commit that referenced this pull request Jun 23, 2026
* feat(core): implement the prop default + custom attribute options

Both were documented but not consumed. Implement on the shared prop
descriptor (so both the bare `{ count: Number }` and `prop(Number, {...})`
forms flow through it): a `default` option (value or per-instance function,
applied at construction, overridden by an applied attribute), and a custom
`attribute` name (honored in observedAttributes, reflection, and the
attribute->property mapping, lit-parity). Tests cover both + a counterfactual.

Refs #575

Claude-Session: https://claude.ai/code/session_01WnvcTojG7tYqmnmf4enSv3

* docs: teach constructor for prop defaults; keep default as lit-parity aside

The default + custom attribute options are now implemented, but defaults are
documented via the constructor (the recommended webjs idiom), not the default
option. Sweep all surfaces (AGENTS, agent-docs/components +
lit-muscle-memory-gotchas, docs site components/task/troubleshooting, blog +
scaffold rule files) to constructor-set defaults, keep a one-line lit-parity
note that the default option also exists, and update the no-longer-static
-properties error message to point at the constructor.

Refs #575

Claude-Session: https://claude.ai/code/session_01WnvcTojG7tYqmnmf4enSv3

* docs: steer webjs check rule messages to constructor defaults + test falsy default

Self-review round 1: the no-static-properties / reactive-props-no-class-field
rule messages (and the fix text) in check.js still told agents to set defaults
via the `default` option, contradicting the constructor-first sweep. Point
them at the constructor. Add a falsy-default test (0 / false / '') pinning the
`!== undefined` guard.

Refs #575

Claude-Session: https://claude.ai/code/session_01WnvcTojG7tYqmnmf4enSv3

* fix(core): resolve a custom prop attribute to its property at SSR

applyAttrsToInstance mapped a source attribute to its property by name /
camelCase only, ignoring the custom `attribute` option, so a parent-rendered
<el is-open> left the real prop unset in the SSR'd first paint (the client
attributeChangedCallback self-corrected after hydration, but the JS-off
contract was wrong). Resolve via the same (d.attribute || hyphenate(k)) rule
the client uses. SSR tests cover default-in-HTML + custom-attribute inbound +
a counterfactual.

Refs #575

Claude-Session: https://claude.ai/code/session_01WnvcTojG7tYqmnmf4enSv3

* fix(intellisense): derive attr completions from the custom attribute option

extractStaticProperties always kebab-cased the property name for the HTML
attribute, so a prop with a custom `attribute` option got the wrong name in
template attribute completions / hover. Read the option (propAttrName), matching
the runtime. Re-vendored the nvim copy src; test covers a custom-attribute
completion.

Refs #575

Claude-Session: https://claude.ai/code/session_01WnvcTojG7tYqmnmf4enSv3

* test(core): pin that a constructor assignment overrides the default option

Lock the precedence (default applied in super() is the fallback; a constructor
assignment after super() wins) so a future change cannot silently flip it.

Refs #575

Claude-Session: https://claude.ai/code/session_01WnvcTojG7tYqmnmf4enSv3

---------

Co-authored-by: t <t@t>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Research a simpler reactive-prop DX than the static-properties + declare pattern

1 participant